internal/image/validate.go (1)

109-119: ⚡ Quick win

Avoid building the same policy input twice per image.

BuildInput already marshals/unmarshals the full payload, and WriteInputFile then marshals it again immediately afterward. On the conftest path this is pure overhead in a hot loop, and it also creates two places that must stay semantically identical. Prefer a single shared builder, or only populate ParsedInput when an OPA evaluator is actually present.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/image/validate.go` around lines 109 - 119, The code currently calls
BuildInput and then WriteInputFile, causing duplicate marshaling; instead call
BuildInput once and reuse its result when writing files or populating
ParsedInput. Modify WriteInputFile (or add an overload) to accept the
already-built inputJSON/inputMap from BuildInput so it does not re-marshal, and
only populate a.ParsedInput (or run the OPA/conftest path) when an OPA
evaluator/conftest is actually present; update callers to pass the BuildInput
output into WriteInputFile or skip WriteInputFile when no evaluator is used
(referencing BuildInput, WriteInputFile, and ParsedInput in your changes).

internal/evaluator/opa_evaluator.go (1)

243-267: ⚡ Quick win

Cache rule discovery per namespace after compilation.

This rescans every compiled module for every (namespace, input) pair, then does an O(n²) dedupe on top. With many inputs, rule discovery becomes part of the hot path even though o.engine.Modules() is immutable after compileEngine(). Precomputing a map[string][]string once during initialization would remove that repeated AST walk and simplify queryNamespace.

As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/evaluator/opa_evaluator.go` around lines 243 - 267, Precompute and
cache discovery of OPA failure/warning rule names per namespace during engine
initialization (e.g., in compileEngine or immediately after it) instead of
recomputing inside queryNamespace by walking o.engine.Modules() each time; build
a map[string][]string (namespace -> ruleNames) using the same filtering logic
(isOPAFailure/isOPAWarning and dedupe) and store it on the evaluator struct,
update usages of ruleNames/ruleCount in queryNamespace to read from that cache,
and ensure cache is populated once after compileEngine() so o.engine.Modules()
is not rescanned for every (namespace, input) pair.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

internal/evaluator/evaluator_comparison_test.go (2)

130-140: ⚡ Quick win

Local variable os shadows the os package import.

Inside assertSameOutcomes, os := summarizeOutcomes(opaResults) shadows the imported os package. It's harmless today (no os.* use in this function) but is confusing and a footgun for future edits. Rename the local to e.g. oo / opaSummary.

Suggested fix

-func assertSameOutcomes(t *testing.T, label string, conftestResults, opaResults []Outcome) {
-	t.Helper()
-	cs := summarizeOutcomes(conftestResults)
-	os := summarizeOutcomes(opaResults)
-
-	assert.Equal(t, cs.failureCodes, os.failureCodes, "%s: failure codes differ", label)
-	assert.Equal(t, cs.warningCodes, os.warningCodes, "%s: warning codes differ", label)
-	assert.Equal(t, cs.successCodes, os.successCodes, "%s: success codes differ", label)
-	assert.Equal(t, cs.failureMsgs, os.failureMsgs, "%s: failure messages differ", label)
-	assert.Equal(t, cs.warningMsgs, os.warningMsgs, "%s: warning messages differ", label)
-}
+func assertSameOutcomes(t *testing.T, label string, conftestResults, opaResults []Outcome) {
+	t.Helper()
+	cs := summarizeOutcomes(conftestResults)
+	oo := summarizeOutcomes(opaResults)
+
+	assert.Equal(t, cs.failureCodes, oo.failureCodes, "%s: failure codes differ", label)
+	assert.Equal(t, cs.warningCodes, oo.warningCodes, "%s: warning codes differ", label)
+	assert.Equal(t, cs.successCodes, oo.successCodes, "%s: success codes differ", label)
+	assert.Equal(t, cs.failureMsgs, oo.failureMsgs, "%s: failure messages differ", label)
+	assert.Equal(t, cs.warningMsgs, oo.warningMsgs, "%s: warning messages differ", label)
+}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/evaluator/evaluator_comparison_test.go` around lines 130 - 140, The
local variable os in assertSameOutcomes shadows the imported os package; rename
it (e.g., opaSummary or oo) and update all subsequent uses (the comparisons of
failureCodes, warningCodes, successCodes, failureMsgs, warningMsgs) to reference
the new variable; locate the variable created by calling
summarizeOutcomes(opaResults) inside assertSameOutcomes and change that
identifier and its uses so the os package import is no longer shadowed.

---

130-140: ⚡ Quick win

Exception messages are summarized but never compared.

summarizeOutcomes populates exceptionMsgs, but assertSameOutcomes does not assert on it, so any divergence in exception handling between conftest and OPA goes unnoticed. Either drop the field or include it in the equality checks to actually catch regressions.

Suggested fix

 	assert.Equal(t, cs.failureMsgs, os.failureMsgs, "%s: failure messages differ", label)
 	assert.Equal(t, cs.warningMsgs, os.warningMsgs, "%s: warning messages differ", label)
+	assert.Equal(t, cs.exceptionMsgs, os.exceptionMsgs, "%s: exception messages differ", label)
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/evaluator/evaluator_comparison_test.go` around lines 130 - 140, The
test helper assertSameOutcomes omits comparing the exceptionMsgs field produced
by summarizeOutcomes, so add an assertion comparing cs.exceptionMsgs and
os.exceptionMsgs (e.g., using assert.Equal with the same "%s: exception messages
differ" label) to catch divergences between conftest and OPA; alternatively, if
exceptionMsgs is intentionally irrelevant, remove/populate it consistently in
summarizeOutcomes so it no longer exists as a source of silent differences.
Ensure references to summarizeOutcomes, assertSameOutcomes, and the
exceptionMsgs field are updated accordingly.

features/validate_image_opa.feature (1)

110-119: 💤 Low value

Inline JSON --policy argument is fragile.

Unlike the other scenarios that reference a named ec-policy, this one inlines JSON directly on the command line. The acceptance harness splits args by literal space (strings.Split(args, " ")), so this only works because the JSON has no spaces — any future formatting (e.g. adding a space after : or ,) will silently break the scenario. Consider switching to a named policy configuration like the other scenarios for consistency and robustness.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/validate_image_opa.feature` around lines 110 - 119, The scenario in
features/validate_image_opa.feature inlines JSON into the --policy argument
which is fragile due to the test harness splitting args on spaces; replace the
inline JSON with a named ec-policy like the other scenarios: add a "Given an
ec-policy named \"future-deny\" with" block that contains the same git source
configuration (e.g. the git::https://... future-deny-policy repo), and update
the ec command to use the policy variable reference (e.g. --policy
${future-deny_POLICY}) instead of the inline JSON so the harness passes the
policy reliably.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes